Skip to content

Conversation

@aabs
Copy link
Owner

@aabs aabs commented Dec 5, 2025

No description provided.

mattana added 9 commits December 5, 2025 09:49
…ability hardening

- Introduced a comprehensive feature specification document outlining user scenarios, requirements, success criteria, and edge cases for hardening the ActorSrcGen source generator.
- Created a detailed task list organized by phases and user stories, including setup, foundation, and implementation of determinism, thread safety, and comprehensive testing.
- Established clear acceptance criteria and measurable outcomes for each user story to ensure reliability and testability of the generator.
…nd enhance tests

- Introduced a new diagnostics class with rules for actor definitions, including checks for step methods and entry points.
- Created a coverage workflow in GitHub Actions to ensure code quality and coverage reporting.
- Added helper classes for compilation and snapshot testing to streamline test setup.
- Implemented integration tests for actor generation, ensuring deterministic outputs across multiple runs and varying input orders.
- Developed unit tests for ActorNode and BlockNode, verifying their properties and behaviors.
- Enhanced diagnostic message tests to validate expected outputs for various actor configurations.
- Added generated code snapshots for various actor patterns to ensure consistency in generated outputs.
… safety, enhance stress testing capabilities
- Implemented various integration tests for actor patterns, including single-step, multi-input, and ingest methods.
- Added attribute validation tests to ensure correct usage of actor attributes.
- Created diagnostic snapshot tests to verify proper diagnostic messages for invalid actor configurations.
- Enhanced existing unit tests for cancellation handling in generator methods.
- Updated generator context creation to support cancellation tokens.
- Introduced new test cases for invalid ingest methods and multiple step definitions.
- Verified generated code against expected outputs for various actor configurations.
Copilot AI review requested due to automatic review settings December 5, 2025 23:07
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request implements comprehensive reliability hardening for the ActorSrcGen source generator, transforming it from minimal test coverage (~0-20%) to enterprise-grade quality with 85%+ overall coverage and 100% critical path coverage. The PR follows a Test-Driven Development approach and addresses constitutional violations around immutability, testability, and diagnostic reporting.

Key Changes

  1. Test Infrastructure Expansion: Added comprehensive test suite with 50+ unit tests, 20+ integration tests, and snapshot testing infrastructure using xUnit and Verify
  2. Domain Model Refactoring: Introduced immutable VisitorResult record to enable pure functional visitor design
  3. Test Coverage: Achieved comprehensive coverage of Generator, ActorVisitor, ActorGenerator, and helper utilities through unit and integration tests

Reviewed changes

Copilot reviewed 93 out of 96 changed files in this pull request and generated 49 comments.

Show a summary per file
File Description
tests/ActorSrcGen.Tests/Usings.cs Added global usings for System.Collections.Immutable, VerifyTests, and VerifyXunit to support new test infrastructure
tests/ActorSrcGen.Tests/Unit/*.cs Comprehensive unit test suite covering TypeHelpers, RoslynExtensions, ActorVisitor, Generator, BlockNode, ActorNode, and Diagnostics with 100% critical path coverage
tests/ActorSrcGen.Tests/Integration/*.cs Integration tests for snapshot patterns, stress testing, thread safety, cancellation, determinism, and diagnostic reporting
tests/ActorSrcGen.Tests/Snapshots/**/*.verified.cs Snapshot test verified outputs for various actor patterns (SimpleActor, PipelineActor, MultiInputActor, IngestActor, ReceiverActor, ComplexActor, etc.)
tests/ActorSrcGen.Tests/Helpers/*.cs Test helper utilities for compilation setup, snapshot verification, and test actor factory patterns
ActorSrcGen/Model/VisitorResult.cs New immutable record encapsulating visitor results (actors + diagnostics) to enable pure functional design
ActorSrcGen/Helpers/TypeHelpers.cs Enhanced null safety with nullable annotations, improved IsCollection to recognize ImmutableArray types, defensive null checks
ActorSrcGen/Templates/Actor.tt Added null/empty handler body fallback to prevent template crashes when HandlerBody is empty
ActorSrcGen/Properties/AssemblyInfo.cs Added InternalsVisibleTo for test project access to internal types
ActorSrcGen/IsExternalInit.cs Shim for C# 9 record support on netstandard2.0 target
tests/ActorSrcGen.Tests/ActorSrcGen.Tests.csproj Updated test project with Verify.Xunit 24.0.0, coverage configuration (85% threshold), and proper analyzer reference pattern
ReadMe.md Added Testing and Diagnostics sections with coverage commands and diagnostic reference links
doc/DIAGNOSTICS.md New diagnostic reference documentation for ASG0001-ASG0003 with remediation guidance
Contributions.md Added TDD workflow requirements and coverage threshold expectations (85% overall, 100% critical)
Directory.Build.props Updated Microsoft.SourceLink.GitHub from 1.0.111 to 1.1.1
specs/001-generator-reliability-hardening/*.md Comprehensive specification, research, data model, quick start guide, plan, tasks, and requirements checklist documentation
tmp-lines.ps1 Utility script for line numbering (appears to be development tooling, not production code)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +172 to +180
foreach (var ingest in ingesters)
{
Method = method,
NodeType = NodeType.TransformMany,
HandlerBody = $$"""
async ({{inputTypeName}} x) => {
try
{
return await {{method.Name}}(x);
}
catch(Exception e)
{
LogMessage(LogLevel.Error, $"Error in {{method.Name}}: {e.Message}\nStack Trace: {e.StackTrace}");
return default;
}
}
"""
};
var method = ingest.Method;
var returnsTask = string.Equals(method.ReturnType.Name, "Task", StringComparison.Ordinal) || method.ReturnType.Name.Contains("AsyncEnumerable", StringComparison.Ordinal);
if (!method.IsStatic || !returnsTask)
{
builder.Add(ActorSrcGen.Diagnostics.Diagnostics.CreateDiagnostic(ActorSrcGen.Diagnostics.Diagnostics.ASG0003, method.Locations.FirstOrDefault() ?? Location.None, method.Name));
}
}
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.

Copilot uses AI. Check for mistakes.
Console.WriteLine("Called Synchronously");

var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disposable 'CancellationTokenSource' is created but not disposed.

Suggested change
var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));

Copilot uses AI. Check for mistakes.
var result = visitor.VisitActor(syntaxAndSymbol);
var actor = Assert.Single(result.Actors);

var diagnostics = new List<Diagnostic>();
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The contents of this container are never accessed.

Copilot uses AI. Check for mistakes.
}

return t.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat);
return t?.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat) ?? string.Empty;
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Condition is always not null because of ... is ....

Suggested change
return t?.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat) ?? string.Empty;
return t.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat);

Copilot uses AI. Check for mistakes.
var src = BuildSingleStep("SingleStepActor1", "input");
var output = Generate(src);

Assert.True(output.ContainsKey("SingleStepActor1.generated.cs"));
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inefficient use of 'ContainsKey' and indexer.

Copilot uses AI. Check for mistakes.

TransformBlock<string,string> _Process;

TransformBlock<string,string> _Start;
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field '_Start' can be 'readonly'.

Copilot uses AI. Check for mistakes.
_Start.LinkTo(_Process, new DataflowLinkOptions { PropagateCompletion = true });
}

TransformBlock<string,string> _Finish;
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field '_Finish' can be 'readonly'.

Copilot uses AI. Check for mistakes.
_Start.LinkTo(_Process, new DataflowLinkOptions { PropagateCompletion = true });
}

TransformBlock<string,string> _Process;
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field '_Process' can be 'readonly'.

Copilot uses AI. Check for mistakes.
_Start.LinkTo(_Finish, new DataflowLinkOptions { PropagateCompletion = true });
}

TransformBlock<string,string> _Finish;
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field '_Finish' can be 'readonly'.

Copilot uses AI. Check for mistakes.

TransformBlock<string,string> _Finish;

TransformManyBlock<string,string> _Start;
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field '_Start' can be 'readonly'.

Copilot uses AI. Check for mistakes.
@aabs aabs merged commit 8ab6509 into master Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants